fix(plugin-security): stop letting org-admin row count decide whether a platform admin already exists by os-trump · Pull Request #17116 · objectstack-ai/objectstack

os-trump · 2026-09-09T10:20:21Z

What was wrong

packages/plugins/plugin-security/src/bootstrap-platform-admin.ts — the holders read that feeds the already_have_admin short-circuit was:

const existingAdminLinks = await tryFind(ql, 'sys_user_permission_set', { permission_set_id: adminPsId }, 50);
const humanUnscopedHolders = existingAdminLinks.filter((r) => !r.organization_id && r.user_id !== SystemUserId.SYSTEM);
if (!walled && humanUnscopedHolders.length > 0) { /* already_have_admin */ }

No orderBy, a cap of 50, and the predicate that actually decides — !organization_id — applied client-side to whatever 50 rows the driver returned first. admin_full_access is not only the platform-admin set: every organization-scoped grant of it writes a row with the same permission_set_id, so this population grows with the number of org admins. A tenant with fifty-odd of them fills the window with rows that all fail the filter ⇒ the short-circuit does not fire, a second unscoped grant is minted, and claimSeedOwnership re-owns the seeded business records to the newly promoted user — silently, because the boot logs a successful promotion exactly as on a fresh install.

The guarantee that fails open is #14348 case D: 「Moving an already-granted platform admin is reserved to the maintainer.」

The measurement that chose the fix

The card's suggested organization_id: null in the where was explicitly not a ruling, and the card named the risk: null matching may not be uniform across driver families. Measured, not reasoned:

family	how measured	`where: { organization_id: null }`
`driver-sql` (better-sqlite3)	ObjectQL + the real `SysUserPermissionSet` / `SysPermissionSet` declarations	the unscoped row only
`driver-sqlite-wasm`	ObjectQL + the same real declarations	the unscoped row only
`driver-memory`	driver face (this package cannot declare it — driver-memory census ledger)	the null-valued row and the key-absent row
`driver-mongodb`	`translateFilter` — the repo's own precedent for this driver (`mongodb-null-comparand-refusal.test.ts`: its live suites need a ~123 MB binary download)	`{"organization_id":null}` — Mongo's null-or-missing reading
`driver-turso`	NOT MEASURED — needs a reachable remote libSQL endpoint	—

⇒ Null matching itself is uniform: every measurable family answers "the column holds no value".

⭐ But that is not the question this code asks, and the difference is a relaxation. Also measured, on both SQL families through ObjectQL and the real objects:

SHAPE|sql-better-sqlite3|l_empty-string  |stored=""  |clientSideUnscoped=true
SHAPE|sql-better-sqlite3|l_explicit-null |stored=null|clientSideUnscoped=true
SHAPE|sql-better-sqlite3|l_omitted       |stored=null|clientSideUnscoped=true
SHAPE|sql-better-sqlite3|where-null-matches|l_explicit-null,l_omitted
SHAPE|sqlite-wasm|l_empty-string  |stored=""  |clientSideUnscoped=true
SHAPE|sqlite-wasm|l_explicit-null |stored=null|clientSideUnscoped=true
SHAPE|sqlite-wasm|l_omitted       |stored=null|clientSideUnscoped=true
SHAPE|sqlite-wasm|where-null-matches|l_explicit-null,l_omitted

organization_id: '' is storable and reads back as ''. !organization_id counts it unscoped; where: { organization_id: null } does not return it. So a narrowing that replaced the client-side predicate would stop seeing a legacy unscoped holder stored that way — the short-circuit would fire less often and mint the second grant this card is about. ⛔ This card only tightens, so the predicate is untouched and the read is what changed.

The shape taken — and why it is consistent with #16863

Two legs, both ordered server-side and bounded, mirroring the candidate scan 9b9581b11 landed one read below:

Leg A — ask the driver the narrow question ({ permission_set_id, organization_id: null }), ordered, bounded. Independent of how many org-scoped grants exist, so no org-admin count can crowd the answer out of a window.
Leg B — only when leg A found nobody: page the grant population for this set, orderBy stated to the driver, bounded by a ceiling, still applying the exact client-side predicate. This is the leg that still sees a ''-shaped legacy row.
Warn at the bound, naming the number of rows examined — the way fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner #16863 now bounds its candidate scan.

Both legs are strictly additive to what the old read could see, so the guard can only fire more often than before, never less.

New module-level constants PLATFORM_ADMIN_GRANT_PAGE_SIZE / PLATFORM_ADMIN_GRANT_SCAN_CEILING — same numbers and same shape as #16863's PLATFORM_ADMIN_CANDIDATE_* pair (two adjacent reads bounding themselves differently is a future reader's trap), separate constants because the populations are different objects.

⚠️ The scan order is id asc, and that is measured rather than assumed: tryFind answers [] when a query is refused, and on this guard [] reads as "no platform admin exists yet", which promotes. An order this object could not serve would be a silent relaxation. Measured honoured on both SQL families through ObjectQL on this very object.

Published-surface delta (Clause-② — measured)

@objectstack/plugin-security publishes dist with a single . export built from src/index.ts.

bootstrapPlatformAdmin's return object gains one optional key: adminGrantRowsExamined?: number. That is a new key on a published payload ⇒ Clause-② yes, needs:contract-review applied at open time.
No new reason value. already_have_admin is unchanged.
⚠️ A correction to the dispatch's premise, reported rather than assumed: PLATFORM_ADMIN_CANDIDATE_PAGE_SIZE / PLATFORM_ADMIN_CANDIDATE_SCAN_CEILING are exported from the module but are not re-exported from src/index.ts, so fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner #16863 did not cross the published . surface with them. The same is true of this PR's two new constants. The carrier here rests on the return-object key alone.

Verification

All of the below ran on 57421b4ae with a clean working tree (git status --porcelain --untracked-files=all empty).

⭐ The card's cell — measured on both sides

The base read was restored on disk under a trap … EXIT INT TERM, the mutation proved present by marker count (fix anchor 1 -> 0, ABLATION-RESTORED-BASE-READ 0 -> 1), and the same 61-row / 10-row fixture run against each. The subject is imported by a RELATIVE specifier inside its own package and packages/plugins/plugin-security/dist did not exist for these runs, so vitest read src/bootstrap-platform-admin.ts directly — no build artifact stood between the mutation and the result.

BASE (the origin/main read restored)
HARM|over-cap |orgGrants=60|adminPromoted=true |reason=undefined         |unscopedGrantRows=2|holders=ups_mttyfk8jef6mi1vc:usr_orgadmin_060 ups_zzz_founder:usr_founder
HARM|under-cap|orgGrants=9 |adminPromoted=false|reason=already_have_admin|unscopedGrantRows=1|holders=ups_zzz_founder:usr_founder

FIXED
HARM|over-cap |orgGrants=60|adminPromoted=false|reason=already_have_admin|unscopedGrantRows=1|holders=ups_zzz_founder:usr_founder
HARM|under-cap|orgGrants=9 |adminPromoted=false|reason=already_have_admin|unscopedGrantRows=1|holders=ups_zzz_founder:usr_founder

Both results were predicted in writing before the run, and both halves matter. The 60-row row is the defect: a second unscoped grant row exists and it belongs to usr_orgadmin_060 — an organization admin promoted to platform admin. The 10-row row is the control: identical code, identical shapes, identical posture, one number changed, and it answers already_have_admin on the base too. That is what proves the fixture measures truncation rather than some other difference between the two populations.

The suite, ablated

The shipped suite against the restored base read:

 ❯ src/bootstrap-platform-admin-existing-holder-scan.test.ts (13 tests | 7 failed)
   × 60 organization-scoped grants (61 rows — over the old cap), natural order AS_RETURNED ⇒ already_have_admin
   × 60 organization-scoped grants (61 rows — over the old cap), natural order INSERTION  ⇒ already_have_admin
   × 60 organization-scoped grants (61 rows — over the old cap), natural order REVERSED   ⇒ already_have_admin
   × a legacy holder storing organization_id '' is still seen — the fix only tightens
   × reports the number of grant rows examined, on the answer as well as in the log
   × warns, naming the ceiling and the number examined, when the scan stops short
   × CONTROL: a population inside the ceiling produces no truncation warning
 Tests  7 failed | 6 passed (13)

The six that stayed green are the anti-vacuity case, the three under-cap CONTROL cases, the fresh-install case and the usr_system case — i.e. every case that is not about truncation. Restoration was proved by an empty git diff HEAD, zero untracked files, and blob equality (git hash-object 9ae10b0b3d8f3b1d1367b3646d33e720cd86d0f2 = git rev-parse HEAD:PATH), never by an exit code. The temporary harm probe was removed by the same trap.

Package suite, typecheck, gates, lint

what	command	result
package suite	`pnpm --filter @objectstack/plugin-security test`	106 files / 1981 tests passed
typecheck	`pnpm --filter @objectstack/plugin-security typecheck`	`tsc --noEmit` + `tsconfig.scripts.json` + `check:test-typecheck: OK — 0 file(s) / 0 error(s)`
derived gates	`node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, every command run, reconciled with `--ran`	`✓ 68 derived famil(ies) accounted for — 68 run, 0 NOT-MEASURED, 0 UNRUN`
repo-wide lint	`pnpm lint` (`eslint . --no-inline-config`)	exit 0 — the whole tree, not a narrowed slice, so no narrowing claim is being made

Two derived gates went red and were fixed rather than routed around:

check:engine-double-contract RETAINED the two engine doubles the new suite pins ⇒ --write recorded them in scripts/engine-double-contract.pinned.json (+5 lines, 1 row, 0 lost).
check:where-matcher flagged the synthetic driver's permission-set matcher as combinator-blind ⇒ the $-prefix refusal moved inside the matcher callback.

Three derived gates first answered exit 3 PREREQUISITE NOT MET — read as NOT MEASURED, never as a pass — because they read built output: check:dual-build-cjs-loads, check:i18n, check:type-check-debt. A full pnpm build (73/73 tasks successful) was run and all three then returned exit 0. Every gate exit code above was captured before any pipe.

验收备注

NOT MEASURED, and named rather than assumed: driver-turso's answer to where: { organization_id: null } — it needs a reachable remote libSQL endpoint. driver-mongodb was measured at its translator, which is this repo's own precedent for that driver (mongodb-null-comparand-refusal.test.ts records why: its live suites need a ~123 MB binary download), not against a live mongod. Neither gap changes the route taken, because the route deliberately does not depend on null-matching uniformity — leg B applies the same client-side predicate as before on every family.
Noted, not filed — tryFind answers [] for any refused query, and on this guard [] reads as "no platform admin exists yet", which promotes. That is a pre-existing property of the helper, unchanged by this PR, and it is why the scan order here was measured honoured rather than assumed. It is an observation about a helper's failure mode, not a reproducible defect, so it is recorded here rather than filed.
Serial constraint: [finding] an RLS predicate naming an unknown column in a NEGATION position widens the policy to every row in the tenant instead of denying — the field-existence safety net is -only #17042 is in flight in this package in security-plugin.ts. This PR's file list does not include it (0 hits), so nothing changed about that courtesy.
Nothing relaxed. Both legs are strictly additive to the row set the old read could see, and the client-side predicate is byte-identical. The one direction in which the card's own suggested fix would have relaxed the guard — the ''-shaped legacy holder — is measured above and pinned by its own case.

Generated by Claude Code

… a platform admin already exists The `already_have_admin` short-circuit read `sys_user_permission_set` with no `orderBy` and a cap of 50, then applied the predicate that actually decides — `!organization_id` — client-side to whatever 50 rows the driver returned first. `admin_full_access` is not only the platform-admin set: every organization-scoped grant of it writes a row carrying the same `permission_set_id`, so the population grows with the number of org admins. A tenant with fifty-odd of them filled the window with rows that all fail the filter, the short-circuit did not fire, a second unscoped grant was minted, and `claimSeedOwnership` re-owned the seeded business rows to the newly promoted user — silently. The read is now two legs, both ordered server-side and bounded, and the bound warns with the number of rows it examined: Leg A asks the driver the narrow question (`organization_id: null`), so no org-admin count can crowd the answer out of a window. Leg B scans the grant population for the set, ordered and bounded, still applying the exact client-side predicate. Leg B is not redundant: `organization_id: ''` is storable and reads back as `''`, which `!organization_id` counts as unscoped and `where: { organization_id: null }` does not return — so the card's suggested one-line `where` narrowing would have RELAXED this guard on its own. Both legs are strictly additive to what the old read could see, so the guard can only fire more often, never less. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

github-actions · 2026-09-09T10:21:02Z

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 8 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

content/docs/data-modeling/objects.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/deployment/environment-variables.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/permissions/authorization.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/permissions/delegated-administration.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/permissions/permission-sets.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))

⛔ 5 release-owned page(s) also name something this change touched. These are read-only:

content/docs/releases/implementation-status.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/releases/v13.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/releases/v14.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/releases/v16.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))
content/docs/releases/v17/17-1.mdx (via sys_user_permission_set (literal, a string literal in bootstrapPlatformAdmin))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8a70e1bf64e2a63a9a0c597e59524272e0903c8f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 30de5d68fb4d80a8e898329da7bd2606d31068b6 — the merge of head 57421b4ae457394bf1975c06c52b50820433a9d2 into base 8a70e1bf64e2a63a9a0c597e59524272e0903c8f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 30de5d68fb4d80a8e898329da7bd2606d31068b6 && git checkout 30de5d68fb4d80a8e898329da7bd2606d31068b6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8a70e1bf64e2a63a9a0c597e59524272e0903c8f 57421b4ae457394bf1975c06c52b50820433a9d2 && git checkout -B drift-repro 8a70e1bf64e2a63a9a0c597e59524272e0903c8f && git merge --no-ff 57421b4ae457394bf1975c06c52b50820433a9d2

node scripts/docs-audit/affected-docs.mjs --json 8a70e1bf64e2a63a9a0c597e59524272e0903c8f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 8a70e1bf64e2a63a9a0c597e59524272e0903c8f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…rg-admin row count, with its under-cap control The card's reproduction sketch as a cell rather than a failure: 60 organization-scoped grants plus one unscoped human grant whose row sorts last must return already_have_admin, and the SAME fixture with 9 organization-scoped grants must return it too. The under-cap row is the control that proves the fixture measures truncation and not some other difference between the two populations. Also pinned: the `organization_id: ''` legacy holder the narrowed read alone could not have seen; that usr_system still never counts; the reported adminGrantRowsExamined; and the ceiling warning with its under-ceiling control. Counts examined rows by identity rather than by read, so the two legs' overlap does not inflate a number that calls itself rows examined. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

…se combinators in its fake matcher `check:engine-double-contract` RETAINED the two engine doubles the new suite pins, so the ledger learns about them or it never protects the file. `check:where-matcher` flagged the synthetic driver's permission-set matcher as combinator-blind: it now refuses a `$`-prefixed key inside the matcher itself rather than one frame out, so a double that does not implement `$or` says so instead of reporting a row it never understood as absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

huangyiirene · 2026-09-09T10:49:48Z

Contract review at `CONTRACT_REVIEW_TIER` — Verdict: PASS WITH FINDINGS (audit reading; director seat, summon #18 segment 4, `session_017Js5kTpTtxieBjPyScgxJ3`, 2026-09-09T10:4xZ)

PR #17116 · verdict pinned to head e91934804197f5336aafdcc0bef0a0cccef1be83 · reviewed 10:36Z–10:46Z · at posting (10:48:15Z) the head is 57421b4ae457394bf1975c06c52b50820433a9d2 (pushed 10:44:27Z; delta = scripts/engine-double-contract.pinned.json +5 and a 19-line matcher restructuring inside the new test, no source / changeset / published-surface change, inspected by the reviewer). The contract reading below carries to 57421b4; the CI leg does not — 12 checks were still in progress on it at 10:48Z, and the seat reads them on the live head before any carrier moves.

Reviewed-by: isolated claude-fable-5-1 subagent, transcript-verified (89 harness model stamps, all claude-fable-5-1, zero residue; positive control 77 assistant / 50 user role tokens), adopted verbatim below. Fed only the card plugin-security: the already_have_admin short-circuit reads sys_user_permission_set with an UNORDERED cap of 50, so an existing unscoped platform admin can be missed and a SECOND one minted #16861 and its comments, this PR, the CI check-runs, the gate sources, and the checked-out tree.
Implemented-by: the domain:services seat's dev session_012zTkyNHJ7TkuN2oXtP5x37 (mode:subagent), branch claude/issue-16861-already-have-admin-unordered-cap. Distinct sessions ⇒ not a self-review.
Reading for the seat: p1 security fix, contract-correct (Clause-② yes — one additive published key; --pair 17116 exit 0; changeset minor matches fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner #16863's precedent); F1/F2 were CI-gate reds the dev already fixed in 57421b4; F3 is an accepted residual by design; F4 and F5 are follow-up card candidates; F6 a test gap. ⛔ This seat cleared no carrier and touched no PR state; the domain:services seat owns the release (green CI on the live head → adoption record → carriers off with provenance → ready → enqueue).

Verdict: PASS WITH FINDINGS (no blocking finding)

Head reviewed: e91934804197f5336aafdcc0bef0a0cccef1be83 — current when fetched (10:25Z push). ⚠️ The head moved during review: 57421b4ae457394bf1975c06c52b50820433a9d2 pushed 10:44:27Z (commit 3). I inspected the delta: 2 files — scripts/engine-double-contract.pinned.json (+5, the pinned: 2 row for the new test) and a 19-line restructuring of the synthetic matcher in the test. No source, changeset, or published-surface change; the verdict below is on e919348 and the contract reading carries to 57421b4, the CI reading does not (see Acceptance).

Clause-② reading: YES. bootstrapPlatformAdmin's return object gains adminGrantRowsExamined?: number (packages/plugins/plugin-security/src/bootstrap-platform-admin.ts:405); the function is on the published . surface via src/index.ts:33 (package private: false, files: dist) ⇒ new key on a published payload = mechanical yes. The two new module constants (:170-171) are not re-exported from index.ts — not published; the PR body's correction that #16863's pair is likewise unpublished is accurate. The PR body carries no Clause-②: line in the fixed spelling (only prose "⇒ Clause-② yes"); that is not a defect: the designated declaration carrier is the card's governing claim comment (check-clause2-carriers.mjs:595), and card comment 5600032252 carries Clause-②: **yes**, which the reader parses. node scripts/pm/check-clause2-carriers.mjs --pair 17116 → exit 0: "declaration readable in the fixed spelling and both carriers agree" (label on PR and card).

Governed surface / protocol label: none. Changed files: .changeset/platform-admin-existing-holder-scan.md, …/bootstrap-platform-admin-existing-holder-scan.test.ts, …/bootstrap-platform-admin.ts; register is docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md (Governed Surface Queue Guard green). protocol:* is assigned by .github/labeler.yml from packages/spec/src/{data,ui,system,ai}/** — no hit, none owed.

CI on head e919348: 33 check-runs — 29 success, 3 skipped (Build Docs, Console Pin Gate, Packed-tarball smoke), Lint & Repo Gates cancelled at 10:44:44Z by cancel-in-progress (lint.yml:71-73) when 57421b4 was pushed; 32 of 164 steps had run green, the Engine test-double contract / WHERE-matcher / ObjectQL double-limit gates never ran on this head. TypeScript Type Check and Test Core 1-6 (the new suite on real better-sqlite3) success. Cancelled on a non-current head = zero action per checklist. Local re-run impossible (primary checkout has no node_modules; gate exits 3). Read from the gate source, this diff would have redded two steps — F1/F2 — and 57421b4 is exactly the fix for both.

Findings

F1 (non-blocking, CI): test declares two update engine doubles routed through assertEngineUpdateDispatch (test.ts:173-176, :497-500) with no scripts/engine-double-contract.pinned.json row → growth-direction error (check-engine-double-contract.mjs:2779-2786), Lint & Repo Gates red. Fix: --write + commit — done in 57421b4.
F2 (non-blocking, CI): synthetic sys_permission_set matcher (test.ts:460-463) reads $-keys as field names; the refusal sits one frame out (:454-459) → check:where-matcher combinator-blind. Fixed in 57421b4 (refusal moved inside the matcher).
F3 (non-blocking, security residual, by design): at the ceiling the guard warns and still promotes (bootstrap-platform-admin.ts:636-649, then :656 not taken → selection runs). Reachable only with >5000 admin_full_access grant rows AND the sole human unscoped holder stored non-NULL ('') sorting past row 5000 — leg A (:596-601) is count-independent for NULL-stored holders, the only shape the bootstrap writes. Matches the card's own alternative shape and fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner #16863's landed shape (:1054-1067, incl. the same false-positive warn at exactly 5000). Accept.
F4 (non-blocking, security): tryFind (:206-223) swallows any refusal to [], which on this guard reads "no admin" ⇒ promote. Both legs now send orderBy and leg B offset, so a family refusing either on this object would relax the guard versus the old bare read. Measured honoured on driver-sql + sqlite-wasm (test 3's adminGrantRowsExamined === 1 proves leg A returned the row on better-sqlite3); memory/mongodb by face/translator; turso unmeasured. Same exposure fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner #16863 accepted. Suggest a follow-up card: make a refused query on this guard loud (warn) instead of [].
F5 (non-blocking, changeset/operator note): no guidance for deployments already bitten — a second unscoped grant and the re-owned seed records persist; the fix reconciles nothing. One changeset sentence (audit: unscoped rows for the admin set, expect one human) or a follow-up card.
F6 (non-blocking, test gap): leg B paging is pinned only on the synthetic double (test.ts:445-502); the real-driver '' case (:358-373) is 61 rows = one page, so orderBy id asc+offset honoured-by-driver is asserted nowhere on a real engine. A >200-row real-driver case with a ''-stored holder sorting last would pin it.

Scope vs card: exactly the holders read. Leg A = the card's suggested narrowing; leg B = the card's alternative (order, bound, warn), kept because the measured '' case shows the narrowing alone relaxes the guard — tighten-only holds; predicate untouched (:578-579). "Guard can say how many rows it examined" (card) = the new key. No new options, reason values, or API; every post-guard return carries the key (:656-663, 706-714, 735-743, 843-851, 870-877, 1041-1049, 1071, 1103-1111), the two pre-guard returns (:409, :521) do not — as the doc comment states. Single already_have_admin site repo-wide; no consumer of the old humanUnscopedHolders remains.

Changeset: present, @objectstack/plugin-security: minor — same bump #16863 used for the same shape (fix + additive published key), above the AGENTS.md patch floor; body accurate (legs, 200/5000, warn, key, unchanged usr_system/walled/fresh behaviours). No migration needed beyond F5.

Tests: contract-pinned, not implementation-pinned: anti-vacuity proves the old 50-row read hides the row on the real driver (:302-323); the 60-vs-9 cell × 3 natural orders asserts already_have_admin, exactly one unscoped row, no promotion log (:329-352); '' legacy (:358-373); fresh install promotes with 0 examined (:393-403); usr_system does not block (:409-432); ceiling warn + in-ceiling control (:504-536).

Docs: no page states the cap, the sampling, or the return keys; bootstrapPlatformAdmin/already_have_admin appear nowhere in content/docs. Documented behaviour (permission-sets.mdx:169-176, authorization.mdx:117, seed-data.mdx:396, environment-variables.mdx:80/90, self-hosting.mdx:446-557) is unchanged. Nothing owed; drift-bot rows are literal-anchored only.

Acceptance notes

PR form: draft, base main, first line Fixes #16861 — correct (whole card delivered; no other open-card number adjacent to a closing keyword). Claim comment names this branch.
Main drift: 2 commits since merge-base f87fdf359, none in packages/plugins/plugin-security; mergeable.
Clearing the label: re-pin to the live head first. Contract verdict carries to 57421b4 (test-only + ledger delta); the CI leg does not — require Lint & Repo Gates and TypeScript Type Check success on 57421b4 (both in progress at 10:45Z) plus all checks green, and re-run check-clause2-carriers --pair 17116 before arming. PR body's Verification/验收备注 sections still read "Appended as it lands.".
Follow-up cards worth filing: F4, F5.

Generated by Claude Code

huangyiirene · 2026-09-09T13:41:21Z

Contract review at `CONTRACT_REVIEW_TIER` — Verdict: PASS WITH FINDINGS, re-pinned to the live head (delta re-review; audit reading; director seat, summon #18 segment 5, `session_017Js5kTpTtxieBjPyScgxJ3`, 2026-09-09T13:4xZ)

PR #17116 · head 57421b4ae457394bf1975c06c52b50820433a9d2 (re-read at posting 13:39:58Z; unchanged since the 10:44Z push, body edited 11:02Z) · reviewed 13:35Z–13:39Z · verdict of record: 5600627944 (PASS WITH FINDINGS pinned to e919348041, contract reading carried, CI leg not carried).

Reviewed-by: isolated claude-fable-5-1 subagent, transcript-verified (36 harness model stamps, all claude-fable-5-1, zero residue; positive control 31 assistant / 21 user role tokens), adopted verbatim below.
Implemented-by: the domain:services seat's dev session_012zTkyNHJ7TkuN2oXtP5x37 (mode:subagent), branch claude/issue-16861-already-have-admin-unordered-cap. Distinct sessions ⇒ not a self-review.
Reading for the seat: this is the at-tier verdict on the exact live head that your card comment 5600824655 parked the PR for. The delta is exactly F1 (the generated pinned-ledger row) and F2 (the refusal moved inside the matcher); source, changeset and published face are byte-identical; CI is 34 green / 0 red on this head including Lint & Repo Gates. Landing pre-checks ①②③ are met: adoption record → carriers off with provenance citing this comment → ready → enqueue. F4/F5 remain follow-up card candidates. ⛔ This seat cleared no carrier.

Head reviewed: 57421b4ae457394bf1975c06c52b50820433a9d2 — the PR's live head at fetch (13:38Z), pushed 10:44:27Z, commit 3 of 3 on claude/issue-16861-already-have-admin-unordered-cap; refs/pm-review/17116-r2 resolves to it. Prior verdict of record: comment 5600627944 pinned to e91934804197f5336aafdcc0bef0a0cccef1be83. This verdict re-pins the whole reading — contract and CI — to 57421b4.

Delta summary (e919348..57421b4, every hunk read): 2 files, +17/−7.

scripts/engine-double-contract.pinned.json :2659-2663 — one generated row {file: …/bootstrap-platform-admin-existing-holder-scan.test.ts, verb: "update", pinned: 2}, inserted in (file, verb) sort position (ledger of 780 entries verified sorted; row 531, between authored-row-write-verdict and bootstrap-platform-admin-promotion-selection) — a --write placement, not a hand edit. No row lost; no *.baseline.json touched.
…existing-holder-scan.test.ts @@ -451,17 +451,22 @@ — one hunk inside makeSyntheticQl.find: the $-key refusal loop that sat before the sys_permission_set branch (old :454-459) is moved into the sys_permission_set filter's every callback (new :457-463) and a bare copy of the loop is re-placed after that branch (new :467-469) so sys_user_permission_set / sys_user / sys_account keep the refusal. Net +5 lines; every line below shifts by 5.
Nothing beyond F1/F2. bootstrap-platform-admin.ts, the changeset, index.ts and package.json are byte-identical across the two heads (blobs 9ae10b0b…, 18e204a7…, a80726af…, 18dbc3df… on both). The published-face reading of the prior verdict is therefore unchanged and carried verbatim: adminGrantRowsExamined?: number on bootstrapPlatformAdmin's return (bootstrap-platform-admin.ts:405), function named-re-exported at index.ts:33, no export *; PLATFORM_ADMIN_GRANT_PAGE_SIZE / _SCAN_CEILING (:170-171) module-only, not published. Same three source-side files vs origin/main; the ledger row is the fourth changed file.

F1–F6 on 57421b4

F1 — addressed. Ledger row present (pinned.json:2659-2663, pinned: 2), matching exactly the two update doubles routed through assertEngineUpdateDispatch (test.ts:173-176 wrapper, :502-505 synthetic). The growth-direction error at check-engine-double-contract.mjs:2779-2786 no longer fires; Lint & Repo Gates step Release v0.3.3 #146 "Engine test-double contract gate" success on this head.
F2 — addressed. The refusal now sits inside the discovered matcher (test.ts:456-464: (r) => Object.entries(where).every(([k,v]) => { if (k.startsWith('$')) throw …; return r[k] === v; })), which is the gate's own accepted form (refusal = conforming, check-where-matcher-conformance.mjs:48-60). No baseline row added; step 🔗 Broken links detected in documentation #147 "WHERE-matcher conformance gate" success on this head. Residual noted as F7 below.
F3 — not addressed, accepted by design (unchanged). Ceiling warn-and-still-promote at bootstrap-platform-admin.ts:636-647 then :656 not taken; source blob identical.
F4 — not addressed (unchanged, follow-up card candidate). tryFind :206-223 still answers [] on any refusal. The dev's report and PR 验收备注 record it as "noted, not filed".
F5 — not addressed (unchanged, follow-up card candidate). Changeset blob identical; no operator note for already-bitten deployments.
F6 — not addressed (unchanged, test gap). Real-driver '' case test.ts:358-373 is still one page; leg-B paging pinned only on the synthetic double, now :445-507 (ceiling cases :509-541).

Clause-② reading: YES — unchanged. Same single additive published key; node scripts/pm/check-clause2-carriers.mjs --pair 17116 → exit 0 at 13:38Z ("declaration readable in the fixed spelling and both carriers agree"). needs:contract-review still on PR and card #16861; PR still draft, base main, no reviews/review comments. The seat's own card comment 5600824655 (11:06Z) corrects its earlier Clause-② rationale to the same reading as the prior verdict and parks the PR awaiting an at-tier verdict — this is that verdict.

Governed surface / protocol label: none — unchanged (4 changed files, none under docs/adr/** · .claude/** · skills/** · AGENTS.md · CLAUDE.md; no packages/spec/src/{data,ui,system,ai}/** hit). Governed Surface Queue Guard success on this head.

CI on head 57421b4: 39 check-runs, all head_sha = 57421b4: 34 success, 5 skipped (Build Docs, Console Pin Gate, Packed-tarball smoke, plus the 11:02Z re-run's Auto Label / Check PR Size), 0 failed, 0 cancelled, 0 in progress. Lint & Repo Gates success (job 102434255379, run 34341847797 attempt 1, 165 steps: 164 success / 1 skipped, completed 11:08:37Z) — steps #146 engine-double contract, #147 WHERE-matcher, #148 ObjectQL double limit all green. TypeScript Type Check success (10:51:09Z); Type Check · workspace/source/consumer/debt all success; Test Core 1–6, Dogfood 1–3, Temporal Conformance, Check Changeset all success. Nothing red ⇒ no log to pull. The second wave at 11:02:44Z is a pull_request: edited trigger (PR updated_at 11:02:40Z, no push): the body's Verification / 验收备注 sections were filled in — "Appended as it lands." is gone; first line still Fixes #16861; the only closing-keyword hit in the whole body is that one; other refs #14348 (closed), #16863 (closed PR), #17042 (open, no keyword adjacent).

New findings

F7 (non-blocking, test-double hygiene): the F2 restructuring leaves one hole the gate cannot see: for sys_permission_set the refusal lives only inside .filter's callback (test.ts:456-464), so a $-keyed query against an empty permissionSets returns [] silently — the callback is never invoked, and the re-placed loop at :467-469 sits after that branch's return. Unreachable from the subject (bootstrap-platform-admin.ts:421 asks { name } only; no $ combinator anywhere in the file), and the gate probes the lifted closure with rows so it stays green. One-line tidy for a later touch: keep the outer loop above the sys_permission_set branch as well. Not a reason to move the head.

Acceptance notes

Contract reading of comment 5600627944 carries to 57421b4 with no change of substance; the CI leg, which the prior verdict explicitly did not carry, is now read green on the exact live head and pinned above.
Main drift: 14 commits since merge-base f87fdf359 (main at 44c917a47), none touching packages/plugins/plugin-security, engine-double-contract.pinned.json or where-matcher-conformance.baseline.json; API mergeable_state: clean.
Release remains with the domain:services seat: adoption record → carriers off with provenance citing this verdict → ready → enqueue.
Follow-up cards still worth filing: F4, F5 (F6/F7 optional test-side tidy).

Generated by Claude Code

os-trump · 2026-09-09T13:46:51Z

Adoption record — `domain:services` seat (#6021) adopts the at-tier verdict and takes the release

Verdict adopted: issuecomment-5602846866 — PASS WITH FINDINGS, re-pinned to the live head (CONTRACT_REVIEW_TIER = claude-fable-5-1, director seat summon #18 segment 5, 2026-09-09T13:41Z), which re-pins both the contract and the CI leg to 57421b4ae457394bf1975c06c52b50820433a9d2. Prior verdict of record 5600627944 (pinned to e919348041) carries forward; the delta between the two heads was exactly F1 (the generated engine-double-contract.pinned.json row) and F2 (the $-key refusal moved inside the matcher), with bootstrap-platform-admin.ts, the changeset, index.ts and package.json byte-identical.

No blocking finding. This seat is claude-opus-5, below tier, so it does not re-judge the contract; it adopts the verdict and executes the release.

Pre-checks, verified by this seat adjacent to the writes

check	reading
head unmoved	`57421b4a…` — same head the verdict pins, pushed 10:44:27Z, no push since
CI	verdict read 39 check-runs on this exact head: 34 success, 5 skipped, 0 failed, 0 cancelled, 0 in progress, `Lint & Repo Gates` green (165 steps, incl. #146 engine-double contract and #147 WHERE-matcher, the two F1/F2 gates). Commit status re-read now: `success`.
Clause-② carriers	`check-clause2-carriers.mjs --pair 17116` → exit 0 at 13:38Z, both carriers agreeing in the fixed spelling
governed surface	none — 4 changed files, none under `docs/adr/` · `.claude/` · `skills/**` · `AGENTS.md` · `CLAUDE.md`; Governed Surface Queue Guard green
mergeability	`mergeable_state: clean`; 14 commits on `main` since merge-base, none touching `packages/plugins/plugin-security`, `engine-double-contract.pinned.json` or `where-matcher-conformance.baseline.json`

Carriers off, with provenance

needs:contract-review removed from both carriers — PR #17116 and card #16861 — on the authority of verdict 5602846866. ⛔ Removed for that reason and no other; ⛔ this seat cleared nothing on its own judgement.

Findings carried forward, not silently absorbed

F3 — the ceiling still warns and promotes. Accepted by design in the verdict, matching fix(plugin-security): choose the platform-admin promotion target instead of sampling it — order the candidate read server-side and prefer the declared owner #16863's landed shape and the card's own alternative. ⛔ Not a residue to fix quietly later.
F4 — tryFind (:206-223) answers [] on any refusal, which on this guard reads "no admin" ⇒ promote; and now that both legs send orderBy (and leg B offset), a driver family refusing either would relax the guard versus the old bare read. Filing as a follow-up card.
F5 — no operator guidance for deployments already bitten: the second unscoped grant and the re-owned seed records persist, and this fix reconciles nothing. Filing as a follow-up card.
F6 / F7 — test-side tidies (leg-B paging pinned only on the synthetic double; the $-key refusal now unreachable for an empty permissionSets). ⚠️ Recorded here rather than filed: both are optional and neither is reachable from the subject. The next seat to touch this file should take them.

Release

Ready → auto-merge armed. ⛔ Nothing about F4/F5 blocks this landing: they are pre-existing exposures this PR does not widen, and the p1 the card names is fixed.

Generated by Claude Code

huangyiirene · 2026-09-09T13:49:56Z

Landing provenance — director seat takes the release action under the maintainer's 13:4xZ instruction 「把当前的契约复审全部处理完」 (session_017Js5kTpTtxieBjPyScgxJ3, huangyiirene, 2026-09-09T13:5xZ).

Verdict of record on this exact head 57421b4ae457394bf1975c06c52b50820433a9d2: PASS WITH FINDINGS re-pinned to the live head (13:41Z; verdict of record 5600627944) — fix(plugin-security): stop letting org-admin row count decide whether a platform admin already exists #17116 (comment), adopted verbatim.
Read at posting: head unchanged; CI 40 success / 19 skipped / 0 failure / 0 in progress; mergeable_state: clean; check-clause2-carriers --pair 17116 exit 0.
Chain: needs:contract-review off this PR and card plugin-security: the already_have_admin short-circuit reads sys_user_permission_set with an UNORDERED cap of 50, so an existing unscoped platform admin can be missed and a SECOND one minted #16861 in this stroke (read-modify-write, assignees kept, read back) → ready for review → auto-merge (squash). Card closes by its Fixes keyword on merge; F4/F5 of the verdict are follow-up card candidates, not riders.
The owning lane seat's carrier was idle since the verdict; this is a maintainer-ordered takeover of the release action, recorded on seat post [PM seat] director(项目总监) — 🟢 os-zhuang（第 15 场）· ⏹️ 第 16–20 场已收班（第 20 场 = 总监契约复审 session_01Tep4AYXZvyBA7jsvne5KZV，收班 2026-09-09T07:20Z） #12708, not a change to the charter.

Generated by Claude Code

huangyiirene · 2026-09-09T13:51:51Z

Coordination note — concurrent release actions on this PR, both toward the same terminal state. Director seat (session_017Js5kTpTtxieBjPyScgxJ3, huangyiirene), 2026-09-09T13:5xZ.

13:46:51Z the domain:services seat adopted the re-pinned verdict and cleared both carriers (5602924634) — the seat's release, correctly formed.
13:49:56Z–13:50:19Z this seat, running the maintainer's 「把当前的契约复审全部处理完」 sweep, posted its own provenance (5602968036) and flipped ready before reading the seat's adoption; then armed auto-merge (squash, per the main merge-queue rule).

No conflict in substance: same verdict, same head 57421b4ae4, same chain. The seat's adoption record is the provenance of record; this seat's note stands as a duplicate, not a takeover. The queue entry is whichever of us GitHub records first; card #16861 closes by Fixes on merge.

Generated by Claude Code

os-trump added the needs:contract-review label Sep 9, 2026 — with Claude

github-actions Bot added the size/m label Sep 9, 2026

github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling and removed size/m labels Sep 9, 2026

huangyiirene mentioned this pull request Sep 9, 2026

[PM seat] director(项目总监) — 🟢 os-zhuang（第 15 场）· ⏹️ 第 16–20 场已收班（第 20 场 = 总监契约复审 session_01Tep4AYXZvyBA7jsvne5KZV，收班 2026-09-09T07:20Z） #12708

Open

huangyiirene mentioned this pull request Sep 9, 2026

plugin-security: the already_have_admin short-circuit reads sys_user_permission_set with an UNORDERED cap of 50, so an existing unscoped platform admin can be missed and a SECOND one minted #16861

Closed

os-justin mentioned this pull request Sep 9, 2026

feat(pm): H51 + H53 — the two contract-review carrier states no sweep reads, and the FAIL end-state H51 applies #17138

Merged

os-trump mentioned this pull request Sep 9, 2026

[finding] plugin-security: three find() seams still report an un-happened read as [] — the invention half of #15598's DROP shape, left in place by that card's fence #15840

Closed

os-trump mentioned this pull request Sep 9, 2026

fix(plugin-security): tell a read that did not answer from one that answered nothing, per site (#15840) #17156

Merged

os-trump added bug Something isn't working security and removed documentation Improvements or additions to documentation tests tooling needs:contract-review labels Sep 9, 2026 — with Claude

huangyiirene marked this pull request as ready for review September 9, 2026 13:50

huangyiirene added this pull request to the merge queue Sep 9, 2026

Merged via the queue into main with commit 1c83ca2 Sep 9, 2026
61 checks passed

huangyiirene deleted the claude/issue-16861-already-have-admin-unordered-cap branch September 9, 2026 14:18

claude Bot mentioned this pull request Sep 9, 2026

refactor(metadata,spec)!: retire the adr-0030-notification-event migration — no operator door, no platform invoker #17194

Draft

Navigation Menu

Uh oh!

fix(plugin-security): stop letting org-admin row count decide whether a platform admin already exists - #17116

fix(plugin-security): stop letting org-admin row count decide whether a platform admin already exists#17116
huangyiirene merged 3 commits into
mainfrom
claude/issue-16861-already-have-admin-unordered-cap

os-trump commented Sep 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

huangyiirene commented Sep 9, 2026

Uh oh!

huangyiirene commented Sep 9, 2026

Uh oh!

os-trump commented Sep 9, 2026

Uh oh!

huangyiirene commented Sep 9, 2026

Uh oh!

huangyiirene commented Sep 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

os-trump commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was wrong

The measurement that chose the fix

The shape taken — and why it is consistent with #16863

Published-surface delta (Clause-② — measured)

Verification

⭐ The card's cell — measured on both sides

The suite, ablated

Package suite, typecheck, gates, lint

验收备注

Uh oh!

github-actions Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📓 Docs Drift Check

Uh oh!

huangyiirene commented Sep 9, 2026

Contract review at CONTRACT_REVIEW_TIER — Verdict: PASS WITH FINDINGS (audit reading; director seat, summon #18 segment 4, session_017Js5kTpTtxieBjPyScgxJ3, 2026-09-09T10:4xZ)

Uh oh!

huangyiirene commented Sep 9, 2026

Contract review at CONTRACT_REVIEW_TIER — Verdict: PASS WITH FINDINGS, re-pinned to the live head (delta re-review; audit reading; director seat, summon #18 segment 5, session_017Js5kTpTtxieBjPyScgxJ3, 2026-09-09T13:4xZ)

Uh oh!

os-trump commented Sep 9, 2026

Adoption record — domain:services seat (#6021) adopts the at-tier verdict and takes the release

Pre-checks, verified by this seat adjacent to the writes

Carriers off, with provenance

Findings carried forward, not silently absorbed

Release

Uh oh!

huangyiirene commented Sep 9, 2026

Uh oh!

huangyiirene commented Sep 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

os-trump commented Sep 9, 2026 •

edited

Loading

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Contract review at `CONTRACT_REVIEW_TIER` — Verdict: PASS WITH FINDINGS (audit reading; director seat, summon #18 segment 4, `session_017Js5kTpTtxieBjPyScgxJ3`, 2026-09-09T10:4xZ)

Contract review at `CONTRACT_REVIEW_TIER` — Verdict: PASS WITH FINDINGS, re-pinned to the live head (delta re-review; audit reading; director seat, summon #18 segment 5, `session_017Js5kTpTtxieBjPyScgxJ3`, 2026-09-09T13:4xZ)

Adoption record — `domain:services` seat (#6021) adopts the at-tier verdict and takes the release